-
Notifications
You must be signed in to change notification settings - Fork 9.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
new_audit: paste-preventing-inputs #14313
Conversation
"description": "Preventing input pasting undermines good security policy (in the case of passwords) and is a UX anti-pattern (in the general case). [Learn more about user-friendly input fields](https://web.dev/password-inputs-can-be-pasted-into/).", | ||
"score": null, | ||
"scoreDisplayMode": "error", | ||
"errorMessage": "Required Inputs gatherer did not run." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adamraine how should I update these FR sample artifacts? We may need a script to do this (like we have for legacy runner)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yarn update:flow-sample-json --rebaseline-artifacts Inputs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah maybe the script needs updating/hardening for this specific use case, but the structure is already there.
6245f6e
to
93b3ff7
Compare
@@ -6,140 +6,160 @@ | |||
"entryType": "measure", | |||
"startTime": 0, | |||
"duration": 1, | |||
"detail": null | |||
"detail": null, | |||
"gather": true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adamraine are these expected changes? From running update on just Inputs
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm looks like a bug in update-flow-fixtures.js
. Timing
should only update if you specify it.
I'll look into it but we can probably still include it in this PR since that's the artifact we would be storing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never mind, I think this is expected. loadArtifacts
adds it when we pull the artifacts from disk:
lighthouse/core/lib/asset-saver.js
Lines 88 to 90 in e154f3d
// Any Timing entries in saved artifacts will have a different timeOrigin than the auditing phase | |
// The `gather` prop is read later in generate-timing-trace and they're added to a separate track of trace events | |
artifacts.Timing.forEach(entry => (entry.gather = true)); |
I think it makes sense to keep.
8a6ef7d
to
de0069f
Compare
Asked ChatGPT what it thinks we should name the audit...
Response:
|
ooooo, pasteability, look at chatgpt go :D |
sorry chatgpt but we went with |
Removes
password-inputs-can-be-pasted-into
and adds a more genericpaste-preventing-inputs
audit.